Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix Model.create failing when the last argument is falsy #13491

Closed

Conversation

MohOraby
Copy link
Contributor

@MohOraby MohOraby commented Jun 9, 2023

Summary

Related to #13487

You shouldn't be able to pass falsy values to Model.create() after doc because it will create a document with empty values (fails schema validation later on)

Added better error handling

The code only checks if options(second argument) and third arguments are callbacks and doesn't take last into consideration

Examples

cases like

const user = await User.create( { name: 'user 1' }, { name: 'user 2' }, { name: 'user 3' }, () => console.log('test'), { name: 'not created' } )
would pass this validation (fails later on)

and a case like const user = await User.create(() => console.log('test')) throws the same cannot read length of undefined error instead of throwing no callbacks error

@MohOraby MohOraby changed the title fix Model.create failing when the last argument is falsy Draft: fix Model.create failing when the last argument is falsy Jun 9, 2023
@MohOraby MohOraby changed the title Draft: fix Model.create failing when the last argument is falsy fix Model.create failing when the last argument is falsy Jun 9, 2023
@vkarpov15
Copy link
Collaborator

Sorry we didn't see this PR, we put in an alternative fix #13493 that doesn't throw an error when you do Model.create(doc, undefined). Will close this PR for now.

@vkarpov15 vkarpov15 closed this Jun 9, 2023
@MohOraby MohOraby deleted the fix/create-fail-undefined branch June 21, 2023 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants